Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap to from js-yaml to yaml for Parsing YAML #1227

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pjkaufman
Copy link
Collaborator

@pjkaufman pjkaufman commented Nov 22, 2024

Fixes #1082 #912 #992 #660 #632 #649

The following are a list of tickets that are potentially fixable with the merger of this change, but logic will need to change in order to properly handle these scenarios:

Relates to #715 #1217 #617

The main reason for making this change is getting closer to having a round trip parser for the Linter to use on the YAML. This change helps with supporting more YAML syntax as well as having a less buggy experience for end users. It should result in fewer instances where things get broken.

This can be better in the long run for users. However it does bring with it some drawbacks that users may not like:

  • It seems that arrays have a space added after the opening [ in a single line array and have a space added before ] at the end of a single line array
  • It seems that multi-line arrays that are empty, but have a place holder entry will get updated to be a little odd
FFF:
  - 

becomes

FFF:
  
  - 

This does not seem to change its meaning in Obsidian which should be fine. But the regex will likely no longer find it. So we may need to update that as well to use this before it can be merged.

  • The Linter can no longer preserve the YAML exactly as it was where the YAML parser is used because there are some instances where the toString logic on the parsed values results in different values from what was originally present

@pjkaufman pjkaufman added bug Something isn't working enhancement New feature or request yaml YAML related issues or features labels Nov 22, 2024
@pjkaufman pjkaufman self-assigned this Nov 22, 2024
@pjkaufman
Copy link
Collaborator Author

If I am understanding correctly, the regex still works when

FFF:
  - 

becomes

FFF:
  
  - 

So I think it should be fine to proceed with this change. I may want to add some more changes first to see how things go.

@pjkaufman
Copy link
Collaborator Author

I checked again to make sure and it seems it converts the value from

FFF:
  - 

to

FFF:
  
  - 

to

FFF:

  
  - 

which the regex does not support. So it is not actually feasible to swap over until all of the regex calls are fixed. However this also brings in some doubt that the underlying parser also knows that the ending version is the original version of the content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request yaml YAML related issues or features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: YAML Key Sort option removes multi-line content after literal operator |
1 participant